Skip to content

fix: Propagate column overrides through grandchild schemas#330

Merged
Andreas Albert (AndreasAlbertQC) merged 4 commits intomainfrom
fix/grandchild-column-override
Apr 17, 2026
Merged

fix: Propagate column overrides through grandchild schemas#330
Andreas Albert (AndreasAlbertQC) merged 4 commits intomainfrom
fix/grandchild-column-override

Conversation

@AndreasAlbertQC
Copy link
Copy Markdown
Collaborator

@AndreasAlbertQC Andreas Albert (AndreasAlbertQC) commented Apr 17, 2026

Motivation

Fixes #329. Starting in 2.8.0, defining a grandchild schema that inherits from a child which overrides a parent column raises ImplementationError: Columns {...} are duplicated with conflicting definitions.:

class Base(dy.Schema):
    amt = dy.Float64(nullable=True)

class Child(Base):
    amt = dy.Float64(nullable=False)

class Grandchild(Child):  # ImplementationError
    pass

SchemaMeta.__new__ drops parent columns shadowed by the current namespace via _remove_overridden_columns, but _get_metadata_recursively — used when walking bases during grandchild creation — never applied the same logic. So both Child's override and Base's original amt landed in the merged metadata and tripped the conflict check.

Changes

  • Call _remove_overridden_columns inside _get_metadata_recursively so column overrides propagate through inheritance chains of any depth.
  • Add regression tests covering grandchild and great-grandchild inheritance of overridden columns.

🤖 Generated with Claude Code

Column overrides defined on a child schema were not shadowing the
parent's definition when traversing bases during grandchild creation,
causing `ImplementationError: Columns {...} are duplicated with
conflicting definitions.` Apply `_remove_overridden_columns` while
walking base metadata recursively so overrides propagate through
inheritance chains of any depth.

Fixes #329

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions github-actions Bot added the fix label Apr 17, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (7c73bb1) to head (76793ba).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #330   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           56        56           
  Lines         3399      3399           
=========================================
  Hits          3399      3399           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

`SchemaMeta.__new__` and `_get_metadata_recursively` were duplicating
the walk-bases / drop-overrides / merge-namespace sequence. Centralise
it in a single helper so both paths stay in sync and future override
semantics only need to change in one place.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a regression in schema inheritance where column overrides defined in an intermediate subclass were not respected when creating deeper descendants (grandchild/great-grandchild schemas), causing conflicting column duplication errors.

Changes:

  • Refactors schema metadata assembly to centralize base/namespace merging in a new helper and applies override-removal during recursive metadata collection.
  • Ensures overridden parent columns are removed while walking inheritance chains, preventing conflicting definitions from accumulating.
  • Adds regression tests covering overridden columns across grandchild and great-grandchild schemas.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
dataframely/_base_schema.py Applies override-removal during recursive metadata collection via a new _collect_metadata helper to prevent conflicts in deep inheritance.
tests/schema/test_inheritance.py Adds regression coverage for overridden columns propagating correctly through grandchild/great-grandchild schemas.

Comment thread dataframely/_base_schema.py
Comment thread dataframely/_base_schema.py Outdated
Widen the `namespace`/`source` parameter types from `dict[str, Any]` to
`Mapping[str, Any]` so that `kls.__dict__` (a `mappingproxy`) can be
passed without a `# type: ignore[arg-type]` cast. Addresses copilot
review feedback on #330.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Approval modulo coverage

@AndreasAlbertQC Andreas Albert (AndreasAlbertQC) merged commit 98fa59e into main Apr 17, 2026
32 checks passed
@AndreasAlbertQC Andreas Albert (AndreasAlbertQC) deleted the fix/grandchild-column-override branch April 17, 2026 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Schema column override breaks in grandchild class (regression in 2.8.0)

3 participants